feat: lazy git clones via artifactory shim#21
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an “artifactory shim” path for non-root git dependencies so Mama can probe/fetch a prebuilt Artifactory package (resolved by git ls-remote commit hash) and skip an expensive clone when the package exists. It also makes shim-based deps read-only (deploy/run/build safeguards) and introduces a marker file ({build_dir}/mama_shim) to persist shim identity across runs.
Changes:
- Add shim marker helpers to
BuildDependencyand a newtry_load_artifactory_shim()probe path inmama.artifactory. - Wire the shim probe into
BuildDependency._load()ahead ofGit.dependency_checkout()and add shim-aware guards for build/deploy/run/open flows. - Add a new test suite covering marker roundtrip, probe gating, load integration (clone skipped vs invoked), and shim guards.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mama/artifactory.py | Adds try_load_artifactory_shim() to probe/fetch via commit hash without cloning. |
| mama/build_dependency.py | Adds shim marker helpers; integrates shim probe before clone; prevents builds/tagging on shims; clears marker on dirty(). |
| mama/build_target.py | Skips deploy/upload for shims; blocks test/start when source is unavailable via _require_source(). |
| mama/main.py | Makes mama open <target> print an actionable hint instead of trying to open a non-existent source dir for shims. |
| mama/papa_deploy.py | Adds defense-in-depth refusal to deploy into a directory containing a mama_shim marker. |
| tests/test_artifactory_shim/init.py | Adds test package for shim-related tests. |
| tests/test_artifactory_shim/test_shim_marker.py | Unit tests for shim marker write/read/detect semantics. |
| tests/test_artifactory_shim/test_shim_probe.py | Unit tests for shim probe gating and fetch hit/miss behavior. |
| tests/test_artifactory_shim/test_shim_load_integration.py | Integration tests ensuring shim hit skips clone and shim miss falls back to clone. |
| tests/test_artifactory_shim/test_shim_guards.py | Tests for build/deploy/tagging/dirty/deploy-destination protections on shims. |
| tests/test_artifactory_shim/test_shim_buildtarget.py | Tests for BuildTarget-level shim behavior (_require_source, deploy short-circuit). |
For non-root git deps with a fresh artifactory build, probe the package
server before cloning. On hit, populate build_dir from the artifactory zip
and write a {build_dir}/mama_shim marker that records the URL/branch/tag/
hash/archive so subsequent runs detect the shim and skip the clone.
The commit hash is resolved via the existing Git.init_commit_hash() which
already supports `git ls-remote` (no working tree needed). When the probe
misses we fall back to the original clone path; `did_check_artifactory`
is intentionally left unset on shim miss so the post-clone fetch can
still pick up a mamafile-declared `target.version`.
Shim is read-only after fetch:
- `_execute_deploy_tasks` short-circuits with a `mama unshallow` hint
- `_execute_run_tasks` refuses `test`/`start` via `_require_source()`
- `mama open <shim>` prints the same hint
- `papa_deploy_to` refuses any destination containing a mama_shim marker
(defense-in-depth for direct callers)
- `_should_build` returns False for shims so configure/build never run
- `update_mamafile_tag` / `update_cmakelists_tag` short-circuit
- `dirty()` removes the marker so the next build re-evaluates
Tests cover the marker roundtrip, probe gating (noart / no artifactory /
non-git / unresolved hash / fetch miss / fetch hit), the load-time wiring
(clone skipped on hit, clone invoked on miss, noart bypass), the guards,
and the BuildTarget-level deploy/require-source behavior. 27 new tests,
all existing tests still pass.
Deferred (will follow up):
- shim → real clone transition for `mama unshallow` (currently users can
`mama wipe` and rebuild, which works via existing semantics)
- explicit `mama update` re-probe path
- `target.version` from src_dir/mamafile.py without a clone (sparse-checkout)
When network is down, the first git ls-remote or HTTP fetch to fail with a clearly network-related error (timeout, DNS, connection refused) sets a global `config._network_available = False`. All subsequent operations check this flag and skip instantly instead of each waiting for their own timeout (previously: N deps × 5s = 50s wasted blocking). Only true network errors trigger the flag. Authentication failures (SSH key rejected, HTTP 401/403) and HTTP 404 are explicitly excluded so that misconfigured credentials don't silently disable all network access. Guard points: - Git.init_commit_hash: skip ls-remote if flag set - Git.fetch_origin: skip fetch/pull if flag set - Git.clone_or_pull: skip pull (use cached source) if flag set; fail clearly if clone needed but network unavailable - _fetch_package: skip HTTP download if flag set The `is_network_error(e)` classifier lives in mama/util.py and handles subprocess.TimeoutExpired, urllib URLError/HTTPError, socket errors, OSError with network errno, and git error message patterns. Behavior by command: - `mama build` with full cache: zero network ops, zero cost - `mama build` first time: first dep times out (5s), flag set, rest fail fast with "network unavailable" message - `mama update`: first dep times out, flag set, rest use cached state https://claude.ai/code/session_01S5uF8PBFkxx5GhSBBTkZvK
6d02139 to
2626195
Compare
…ange on repeat mama update)
…wraps, one-liner ifs)
…n Popen formatting
…nd refactor learnings
…_shim, trim docstrings)
Plain `mama build` (without `update` and without `noart`) now routes through try_load_cached_shim when a valid shim marker exists. This skips both the ls-remote probe and the cached-zip re-extraction that _try_artifactory_shim previously did on every invocation. Update mode still bypasses the cached path so a re-extract happens; noart still ls-remotes for staleness. Captured the build-vs-update contract in docs/build_behavior.md so the intended behaviour does not drift again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For non-root git deps with a fresh artifactory build, probe the package server before cloning. On hit, populate build_dir from the artifactory zip and write a {build_dir}/mama_shim marker that records the URL/branch/tag/ hash/archive so subsequent runs detect the shim and skip the clone.
The commit hash is resolved via the existing Git.init_commit_hash() which already supports
git ls-remote(no working tree needed). When the probe misses we fall back to the original clone path;did_check_artifactoryis intentionally left unset on shim miss so the post-clone fetch can still pick up a mamafile-declaredtarget.version.Shim is read-only after fetch:
_execute_deploy_tasksshort-circuits with amama unshallowhint_execute_run_tasksrefusestest/startvia_require_source()mama open <shim>prints the same hintpapa_deploy_torefuses any destination containing a mama_shim marker (defense-in-depth for direct callers)_should_buildreturns False for shims so configure/build never runupdate_mamafile_tag/update_cmakelists_tagshort-circuitdirty()removes the marker so the next build re-evaluatesTests cover the marker roundtrip, probe gating (noart / no artifactory / non-git / unresolved hash / fetch miss / fetch hit), the load-time wiring (clone skipped on hit, clone invoked on miss, noart bypass), the guards, and the BuildTarget-level deploy/require-source behavior. 27 new tests, all existing tests still pass.
Deferred (will follow up):
mama unshallow(currently users canmama wipeand rebuild, which works via existing semantics)mama updatere-probe pathtarget.versionfrom src_dir/mamafile.py without a clone (sparse-checkout)